RDKOSS-1008: Wait for A2DP disconnect confirmation in BTRMGR_DeInit - #116
Conversation
Reason for change : Added wait time (max 1s) for audio device disconnect confirmation before exiting D-Bus. Without this, btmgr exits while AVDTP CLOSE signaling is still in-flight, causing bluetoothd to crash in clear_endpoint when the response arrives after endpoint destruction. Test Procedure: Regression test cases should be performed. Risks: High Signed-off-by: Natraj <Natraj_Muthusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
This PR updates BTRMGR_DeInit to wait (up to 500ms) for A2DP-related disconnect confirmation before tearing down D-Bus, reducing the chance of bluetoothd crashing when late AVDTP CLOSE responses arrive after endpoint destruction.
Changes:
- After initiating a disconnect for audio devices (speakers/headset), poll for a disconnect-confirmation status for up to 500ms before continuing shutdown.
- Add logging for disconnect-confirmation success/timeout during deinitialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/ifce/btrMgr.c:4131
- The disconnect-confirmation polling block has a few issues: (1) it hard-codes a 1000ms max wait (10×100ms) but the PR description says 500ms, (2) it always sleeps 100ms before the first status check (adding an avoidable delay even when already disconnected) and the reported time can be wrong (often 0ms), and (3) the elapsed value is printed with "%d" even though it’s an unsigned expression (format mismatch/UB). Consider restructuring to check-first then sleep until an explicit max-wait is reached, and log success at INFO/DEBUG rather than WARN.
if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
unsigned int ui32PollCount = 10; /* 10 × 100ms = 1000ms max */
do {
usleep(100000); /* 100ms */
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/ifce/btrMgr.c:4128
- In
BTRMGR_DeInit, the disconnect confirmation loop currently (a) can run even ifBTRCore_DisconnectDevicefails (return value is not stored) and (b) waits up to 700ms (7×100ms), which conflicts with the PR description’s “max 500ms”. The elapsed-time log also uses(10 - ui32PollCount)which reports incorrect durations (e.g., 300ms when only 100ms elapsed). Consider storing the disconnect return, polling only on success, using a 500ms max, and computing the elapsed time from the number of polls performed.
if (BTRCore_DisconnectDevice(ghBTRCoreHdl, lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle, lenBtrCoreDevTy) != enBTRCoreSuccess) {
BTRMGRLOG_ERROR ("Failed to Disconnect - %llu\n", lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle);
}
if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
unsigned int ui32PollCount = 7; /* 7 × 100ms = 700ms max */
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/ifce/btrMgr.c:4133
- The new disconnect-confirmation polling has a few correctness issues: it always sleeps 100ms before the first status check (adding avoidable delay even when the device is already disconnected), the loop never decrements the poll counter on success (so the reported elapsed time can be 0ms even though a sleep already occurred), and it currently waits up to 1000ms despite the PR description stating 500ms. Also, successful confirmation is logged as WARN and the elapsed time uses a signed format specifier.
Consider checking the disconnect status immediately, limiting the wait to 500ms, and logging success at DEBUG/INFO level.
if (lenBtrCoreDevTy == enBTRCoreSpeakers || lenBtrCoreDevTy == enBTRCoreHeadSet) {
unsigned int ui32PollCount = 10; /* 10 × 100ms = 1000ms max */
do {
usleep(100000); /* 100ms */
lenBtrCoreRet = BTRCore_GetDeviceDisconnected(ghBTRCoreHdl,lstConnectedDevices.m_deviceProperty[ui16LoopIdx].m_deviceHandle,lenBtrCoreDevTy);
} while ((lenBtrCoreRet != enBTRCoreSuccess) && (--ui32PollCount));
Reason for change : Added wait time (max 500ms) for audio device disconnect confirmation before exiting D-Bus. Without this, btmgr exits while AVDTP CLOSE signaling is still in-flight, causing bluetoothd to crash in clear_endpoint when the response arrives after endpoint destruction.